Skip to content

Fix AddAnnotationProcessor when parent matches by GAV alone#7527

Open
MBoegers wants to merge 4 commits intomainfrom
mboegers/2288-lombok-repro
Open

Fix AddAnnotationProcessor when parent matches by GAV alone#7527
MBoegers wants to merge 4 commits intomainfrom
mboegers/2288-lombok-repro

Conversation

@MBoegers
Copy link
Copy Markdown
Contributor

Summary

  • Fix orphan modules being silently skipped when their external parent's GAV happens to also exist as a separately ingested LST artifact (MavenResolutionResult.parentPomIsProjectPom() is GAV-only).
  • Verify each parent→child link by walking aggregator <modules> chains; treat the child as an orphan when no aggregator actually links them.
  • Fill in missing <configuration> / <annotationProcessorPaths> when the matched plugin is pre-declared without it (previously AddPluginVisitor left such plugins untouched).

Problem

AddAnnotationProcessor produced zero changes on Maven modules that:

  1. Have no in-reactor parent in their actual build, but coincidentally co-exist in the same LST as a POM that shares their parent's GAV (e.g. an externally-ingested corporate parent), or
  2. Pre-declare maven-compiler-plugin without annotationProcessorPaths (e.g. only to set <source>/<target> or for inherited config).

In case 1, parentPomIsProjectPom() returns true purely because GAVs match, so the child is routed to the in-reactor-parent branch and never added to candidateOrphanPaths. In the visitor it is neither parent nor orphan and is skipped, so its own build/plugins is never updated.

In case 2, AddPluginVisitor's "plugin already exists" branch leaves the plugin untouched, and the path-adding inner visitor only operates on existing <annotationProcessorPaths> tags — so nothing is ever added.

Solution

  1. Reactor-aware parent classification. During scan, record tentativeChildToParent links and aggregator <modules> membership (read off the requested Pom, since ResolvedPom.subprojects is unreliable in some LST fixtures and mrr.getModules() lists POMs claiming this one as <parent> rather than ones aggregated by it). Before the visitor runs, walk aggregator <modules> chains via BFS; uphold a tentative link only when the child is reactor-reachable. Otherwise reclassify as orphan and add the processor to its own build/plugins. Skip dangling packaging=pom POMs that are neither claimed parents nor aggregators.

  2. Bypass for GAV-coincident orphans. Pass the source path as AddPluginVisitor's filePattern only for the GAV-coincident orphan case so its own parentPomIsProjectPom() short-circuit doesn't refuse to add the plugin. Genuine in-reactor parents and true single-module orphans hit the existing acceptance path with filePattern=null.

  3. Fill in missing <configuration> structure. Before the path-adding visitor runs, ensure the matched plugin has <configuration><annotationProcessorPaths/></configuration>. The helper is conservative: only fills in whichever level is missing.

Test plan

  • New orphanModuleSharedLstWithExternalParent reproducer fails on main and passes after the fix.
  • New addConfigurationToExistingPluginWithNoConfig and addAnnotationProcessorPathsToExistingConfiguration reproducers fail on main and pass after the fix.
  • All 30 existing AddAnnotationProcessorTest cases continue to pass.
  • Full :rewrite-maven:test passes.

…ares the LST set

When a Maven module has no in-reactor parent in the actual build but its
corporate parent POM happens to also be present as a separately ingested
LST artifact, AddAnnotationProcessor produces zero changes on the child.
parentPomIsProjectPom() is GAV-based, so it returns true and the child is
routed to the in-reactor-parent branch and never added to
candidateOrphanPaths. In the visitor it is neither parent nor orphan and
is skipped, so its own build/plugins is never updated.

The new test orphanModuleSharedLstWithExternalParent reproduces this with
two independently-rooted mavenProject() blocks (no aggregator between them).
…ents as in-reactor

MavenResolutionResult.parentPomIsProjectPom() answers a GAV-only question:
"is the parent's GAV among the project poms?" Two POMs co-ingested into the
same LST (e.g. an external corp-parent and one of its independently-built
modules) match by GAV without sharing any actual Maven reactor, but the
scanner used the GAV match alone to route the child away from the orphan
branch — resulting in zero changes for the affected module.

Defer the routing decision until aggregator data has been collected, then
verify the parent-child link by walking aggregator <modules> chains. A link
counts only if the child is reachable via some aggregator's reactor BFS;
otherwise the child falls into the orphan branch and gets its own
build/plugins. Read aggregator membership from the requested (raw) Pom's
subprojects, since ResolvedPom.subprojects is unreliable in some LST
fixtures and mrr.getModules() lists POMs that declare *this* one as
their <parent> rather than ones aggregated by it.

Also:
- Skip dangling pom-packaging POMs (not claimed as parents, not aggregators)
  so coincidentally-ingested external parents don't get spurious
  build/plugins.
- Pass the source path as AddPluginVisitor's filePattern only for the
  reactor-broken-orphan case, so its own parentPomIsProjectPom() guard
  doesn't refuse to add the plugin.
- Simplify the reproducer test: drop the pre-declared maven-compiler-plugin
  on the child, since AddPluginVisitor does not currently fill in missing
  configuration on an existing plugin (a separate latent bug; not addressed
  in this commit).
…notationProcessorPaths

When the target POM pre-declares maven-compiler-plugin (e.g. just to set
<source>/<target>) but does not configure annotationProcessorPaths,
AddAnnotationProcessor produces zero changes today: AddPluginVisitor
short-circuits because the plugin already exists, and the inner visitor
only operates on existing <annotationProcessorPaths> tags.

Two new tests in SingleModuleProject reproduce the gap:

- addConfigurationToExistingPluginWithNoConfig — plugin declared with no
  <configuration> at all.
- addAnnotationProcessorPathsToExistingConfiguration — plugin has
  <configuration> with <source>/<target> but no annotationProcessorPaths.

Both expect the recipe to fill in the missing structure. Both fail on the
current main with "Recipe was expected to make a change but made no
changes." Fix follows in a separate commit.
…g maven-compiler-plugin

When the recipe target POM already declared maven-compiler-plugin (e.g.
solely to set <source>/<target>), AddPluginVisitor's "plugin already
exists" branch left the plugin untouched, and the path-adding inner
visitor only operated on existing <annotationProcessorPaths> tags — so
nothing got added. This is the second of the two bugs surfacing on top of
the GAV-coincidence orphan-detection issue.

Before invoking the path-adding visitor on the matched plugin, ensure
<configuration><annotationProcessorPaths/></configuration> exists. The
helper is conservative: it adds nothing when the structure is already
present, only fills in whichever level is missing.
@github-project-automation github-project-automation Bot moved this to In Progress in OpenRewrite Apr 30, 2026
@MBoegers MBoegers marked this pull request as ready for review April 30, 2026 09:57
@MBoegers MBoegers requested a review from timtebeek April 30, 2026 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

1 participant